WifiConnectionManager: PreScan Check for Connect #11039
Conversation
tianocore-assign-reviewers
bot
requested review from
SaloniKasbekar,
ajfish,
ardbiesheuvel,
leiflindholm,
makubacki and
mdkinney
Zclarkwilliams
force-pushed
the
fix-wcm-prescan-check
branch
3 times, most recently
from
b7ed85c to
fcc2b47
Compare
Zclarkwilliams
force-pushed
the
fix-wcm-prescan-check
branch
2 times, most recently
from
fb1507f to
4478381
Compare
|
Is this considered a critical bug fix for the edk2-stable202505 release? |
Yes, This prevents network breaking due to scanning while in connection processing state or no profile provided for connection causing noise and instability. |
Zclarkwilliams
force-pushed
the
fix-wcm-prescan-check
branch
from
4478381 to
91fcd09
Compare
Zclarkwilliams
force-pushed
the
fix-wcm-prescan-check
branch
2 times, most recently
from
4aea932 to
0213064
Compare
|
|
||
| if ((Nic->ConnectPendingNetwork == NULL) || (StrLen (Nic->ConnectPendingNetwork->SSId) < SSID_MIN_LEN)) { | ||
| DEBUG ((DEBUG_INFO, "[WiFi Connection Manager] No valid profile for connection, no scan triggered!\n")); | ||
| gBS->CloseEvent (Event); |
There was a problem hiding this comment.
Why do these error return paths close the event and others in this function do not?
There was a problem hiding this comment.
These failure conditions indicate that the scan is invalid and should not be executed, so we close the event. Others are possible valid scans where there was a hiccup in the system where the scan is still valid but didn't execute fully this round, but should still scan.
There was a problem hiding this comment.
Perhaps some comments to explain why the event is being closed.
There was a problem hiding this comment.
WifiMgrOnTimerTick() is called by a timer event, and IIUC, closing it will prevent it from being called again until the driver unbinds and binds again. Is that really the right thing to do here?
When the Wifi network is enabled the connection manager will trigger a network scan without a profile to use. If there is a connected network or attempting a connection, the scan will interrupt and break the connection. Fix - The Wifi Connection Manager will check for SSID length longer than the minimum SSID length, and if the state is not in connecting or connected. Only of SSID string length is over the defined minimum and state are not connected then it will attempt the network scan as normal. Signed-off-by: Zachary Clark-Williams <[email protected]>
Zclarkwilliams
force-pushed
the
fix-wcm-prescan-check
branch
from
0213064 to
e258cb7
Compare
|
@leiflindholm @ajfish @makubacki @ardbiesheuvel Please review this bug fix for edk2-stable2020505. There is one update outstanding to add comments to explain why CloseEvent() is called. Once those comments are added, the change is complete. |
| gBS->CloseEvent (Event); | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
These early returns from the function make some of the code in the switch below unreachable, so it seems to me that the logic should be combined in some way.
|
On review of the event closure causing possible future issues if a user wants to connect and scan a new profile, this might cause issues. Drafted updated changes to set timer control through timer setting control in HII. |
4 participants
Description
When the Wifi network is enabled the connection
manager will trigger a network scan without a
profile to use. If there is a connected network
or attempting a connection, the scan will
interrupt and break the connection.
Fix - The Wifi Connection Manager will check for
SSID length longer than the minimum SSID length,
and if the state is not in connecting or
connected. Only of SSID string length is over the
defined minimum and state are not connected then
it will attempt the network scan as normal.
How This Was Tested
This change was tested on platforms with Wifi connections established and tested if this fixes the break in the current connect as well if this change breaks anything when the platform is supposed to scan correctly.
Integration Instructions
N/A